Skip to content

feat(backend)!: 30-day default recording retention and THINKER_RECORDING_* names - #148

Merged
patrickbrandao merged 1 commit into
mainfrom
feat/recording-retention-30d
Oct 14, 2026
Merged

patrickbrandao merged 1 commit into
mainfrom
feat/recording-retention-30d

Conversation

@patrickbrandao

Copy link
Copy Markdown
Owner

Summary

  • 30-day default retention. DEFAULT_RECORDING_RETENTION_DAYS = 30 (config/recording-retention.ts). Before this change the default was 365 days. THINKER_RECORDING_RETENTION_DAYS overrides it. A value that is not a positive integer (0, -1, 1.5, 7d, ...) fails boot with a message naming the variable, so a typo can no longer keep recordings forever.
  • Rename, no aliases. The whole REQUEST_RECORDING_* family becomes THINKER_RECORDING_* in app.config.ts, the storage service error message, packages/backend/.env.example, the specs and docs/dev/manifest-guide.md. Old names are never read. If one is still set, boot logs a warning naming its replacement and never its value. This catches a stale S3 config that would otherwise fall back to the filesystem without notice.
  • Boot log (RequestRecordingRetentionService.onApplicationBootstrap, after storage init): Request recordings: retention N days, storage <filesystem|s3|unavailable>.
  • grep -rn "REQUEST_RECORDING_" packages finds nothing.

Not in this PR (owned by other agents, must land together)

The task scoped docker/, the deploy templates and CI out of this PR, but they still set the old names and need the same mechanical rename:

  • docker/docker-compose.yml, docker/.env.example
  • render.yaml, app.json, .do/deploy.template.yaml, deploy/** (fly, gcp, aws, coolify, easypanel, apple-containers, koyeb/heroku/digitalocean/aws tutorials)
  • .github/workflows/docker-smoke.yml

Until they are renamed, those deployments boot with the default storage (auto) and a 30-day retention. The new warning names every ignored variable. S3-only templates (fly, aws, koyeb, heroku, DO) would fall back to filesystem storage. Do not release before the follow-up lands. The rename itself is s/REQUEST_RECORDING_/THINKER_RECORDING_/g.

Tests

  • Unit: recording-retention.spec.ts (new: default 30, override, invalid values, legacy warning without values), app.config.spec.ts (default 30, override 7, invalid fails, old name ignored), request-recording-retention.service.spec.ts (30-day default, override, boot log, legacy warning).
  • E2e test/request-recordings.e2e-spec.ts: with no variable set, a 31-day-old recording is deleted (object first, then pointer; the parent Request is kept) and a 29-day-old one is kept and still readable. After a restart with THINKER_RECORDING_RETENTION_DAYS=7, the 29-day-old one is deleted.

Local runs:

  • npm run lint (0 errors), npm run typecheck, npm run build: pass
  • npx jest (backend): 476 suites, 9097 tests passed
  • TZ=UTC npm run test:e2e:docker --workspace=packages/backend: 71 suites, 600 tests passed

qa-docker: required (runtime behavior changed). Confirm the boot log line and that filesystem recording still works.

Closes #28

🤖 Generated with Claude Code

…ING_* names

Request recordings now expire after 30 days by default
(DEFAULT_RECORDING_RETENTION_DAYS), overridable with
THINKER_RECORDING_RETENTION_DAYS. A value that is not a positive integer
fails boot with a message naming the variable.

The REQUEST_RECORDING_* family is renamed to THINKER_RECORDING_* without
aliases. If an old name is still set, boot logs a warning naming its
replacement (never the value). Boot also logs the effective setting:
"Request recordings: retention N days, storage <filesystem|s3|unavailable>".

Closes #28

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@patrickbrandao

Copy link
Copy Markdown
Owner Author

Review PR #148: 30-day recording retention + THINKER_RECORDING_*

Reviewed git diff origin/main...03dcf160 (12 files).

Blockers

None.

Should-fix (track before merge)

  1. Deploy templates and compose still use REQUEST_RECORDING_*. Files: docker/docker-compose.yml, docker/.env.example, render.yaml, app.json, .do/deploy.template.yaml, deploy/fly/fly.toml, deploy/gcp/main.tf, deploy/easypanel/**, deploy/digitalocean/TUTORIAL.md. Scenario: someone deploys from a template after this merges. The S3 or filesystem path settings are then ignored silently, apart from the boot warning, and storage falls back to auto. Known gap. Fix in the repo-wide rename PR, and record the tracking issue link in this PR before merge.
  2. Upgrade deletes older recordings. An instance on the old 365-day default loses every recording older than 30 days at the next 02:00 cron run. This is the intended decision (decisoes-iniciais §4), and docs/releases/0.1.0.md must say so explicitly, together with how to keep the old behavior (THINKER_RECORDING_RETENTION_DAYS=365). Check that it does before the release.

Nits

  • packages/backend/test/request-recordings.e2e-spec.ts: the temp dir prefix manifest-recordings-e2e- is legacy (pre-existing). Rename it to thinker- in the cleanup.

Checks

  • Retention: strict parsing, and an invalid value fails boot. The cron uses the value without a null fallback. e2e covers 31d/29d at the default and 7 after a restart.
  • Legacy warning: logs names only, never values (unit-tested with a secret value). No credentials are logged.
  • No new outbound calls, no manifest identifiers added, Conventional title, changeset present.
  • No high-risk criterion: no migration, crypto, auth or tenant scoping change. The retention change applies a decision already recorded.
  • CI on the SHA: all 19 check runs succeeded. qa-docker must still pass for the runtime change (merge gate).

Under aprovado, each should-fix must be fixed or tracked in an issue whose link is recorded in the PR before merge.

Head revisado: 03dcf16
Veredito: aprovado

@patrickbrandao

Copy link
Copy Markdown
Owner Author

Review should-fixes 1 and 2 are tracked in #152. The deploy templates are renamed in the repo-wide rename PR; the 30-day upgrade note goes into docs/releases/0.1.0.md.

@patrickbrandao

Copy link
Copy Markdown
Owner Author

qa-docker report

Image thinker:qa-26579 (arm64, Docker Desktop), built from docker/Dockerfile in about 4m55s, 429 MB. Bases: node:24-alpine, node:24-slim, gcr.io/distroless/nodejs24-debian13. Postgres: postgres:16-alpine. Mock provider only, THINKER_CATALOG_SYNC=false, run-scoped volumes (override with name: !reset null, labels verified, 0 left after teardown).

Step Result
Image build pass
Stack boot and /api/v1/health pass (7s)
Smoke: dashboard HTML, first admin, agent key, /v1/models pass
Chat completion: OpenAI, Anthropic (/v1/messages), Responses (/v1/responses) pass
Backend e2e (test:e2e:docker) pass: 71 suites, 600 tests
Gateway contracts pass: 9/9
Check 1: boot line "Request recordings: retention 30 days, storage filesystem" pass
Check 2: recording written to the filesystem volume (3 .json.gz files, matching recording_key rows) pass
Check 3: THINKER_RECORDING_RETENTION_DAYS=abc pass: exit 1, "THINKER_RECORDING_RETENTION_DAYS must be a positive integer number of days (got "abc")..."
Check 4: legacy REQUEST_RECORDING_* warning pass: names REQUEST_RECORDING_X -> THINKER_RECORDING_X, value SUPERSECRETVAL99 never logged
Teardown pass

Finding (should-fix, not a failure of the checks): docker/docker-compose.yml and docker/.env.example still use REQUEST_RECORDING_*. With the stock compose file the container always receives REQUEST_RECORDING_STORAGE, _FILESYSTEM_PATH and _S3_FORCE_PATH_STYLE, so every Docker boot logs the legacy warning, and a user setting REQUEST_RECORDING_RETENTION_DAYS in .env is silently ignored (with only the legacy warning in the logs). Recordings still landed on the mounted volume, so default storage works. Update compose and .env.example to THINKER_RECORDING_* in this PR or in a ticketed follow-up.

Other notes: nothing flaky. A Better Auth "Database schema mismatch" line appears once at first boot, before migrations. Not related to this PR.

Tested SHA: 03dcf16

@patrickbrandao
patrickbrandao marked this pull request as ready for review October 14, 2026 13:24
@patrickbrandao
patrickbrandao merged commit 16a51fd into main Oct 14, 2026
19 checks passed
@patrickbrandao
patrickbrandao deleted the feat/recording-retention-30d branch October 14, 2026 13:25
patrickbrandao added a commit that referenced this pull request Oct 14, 2026
* feat(cli)!: rename the CLI to thinker with THINKER_* variables

- Package thinker-cli (private, no publishConfig), bin thinker -> bin/thinker.js.
- Env vars THINKER_URL, THINKER_API_KEY, THINKER_AGENT_KEY, THINKER_AGENT_URL;
  the old names are not read (no aliases).
- Config and agent-key store under ~/.config/thinker/; default origin stays
  http://localhost:2099. Fix the stale "then Cloud" resolution comment.
- User-Agent thinker-cli/<version>; usage, hints and messages say thinker.
- Embedded skill renamed thinker-cli and rewritten for a self-hosted Thinker
  instance; regenerated skill-content.gen.ts.
- routing test detects the gateway's peacock error tag for any product word,
  so it keeps working across the server-side rename of that tag.
- README: local install (npm install -g ./packages/cli or npm pack) and a note
  that the upstream CLI's config directory (including leftover telemetry
  files) is neither read nor migrated.
- Root: changesets ignore list and check-changesets message use thinker-cli;
  lockfile updated; dev guide CLI section refreshed.

BREAKING CHANGE: the CLI command, environment variables and config directory
are renamed; run thinker login again.

Refs #23

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* feat(docker)!: rebrand image, compose and installer to Thinker

- docker-compose.yml: project/service `thinker`, Postgres user/db
  `thinker`, volumes thinker_pgdata and thinker_request_recordings (no
  legacy pins), `# thinker-compose-version: 1`, no legacy 3001 section,
  TRUST_PROXY exposed with a `loopback` default so clients on a private
  network cannot spoof X-Forwarded-For to dodge the per-IP rate limit.
- install.sh: THINKER_INSTALLER_SOURCE / THINKER_INSTALLER_SKIP_PULL (no
  aliases), raw source patrickbrandao/thinker, default dir $HOME/thinker,
  thinker compose marker, `docker compose logs -f thinker`, LAN note via
  HOST_BIND_ADDRESS.
- Dockerfile: Thinker OCI labels, drop the stale base.digest label,
  ENV PORT=2099 and healthcheck fallback 2099, ship /app/LICENSE and a
  THIRD_PARTY_NOTICES generated offline before node_modules is pruned.
- Backend: single DEFAULT_PORT = 2099 constant replaces every 3001
  fallback; spec fixtures follow.
- DOCKER_README.md: minimal rebrand (no upstream stars badge, API Bot
  banner or manifest.build links).
- docker-smoke.yml: drop the Compose override and the patched compose
  copy; tag the local build tmsoftbrasil/thinker:ci and select it with
  THINKER_VERSION=ci (restores the image-tag check), new service, volume
  and container names, and check the installer writes two distinct
  secrets. docker.yml checks LICENSE, THIRD_PARTY_NOTICES and PORT=2099.

BREAKING CHANGE: MANIFEST_INSTALLER_* are no longer read; the Compose
project, service and volume names changed.

Refs #141

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* feat(n8n)!: rebrand the n8n node as n8n-nodes-thinker

- git mv integrations/n8n-nodes-manifest -> integrations/n8n-nodes-thinker;
  package n8n-nodes-thinker 0.1.0, repository/homepage/author point at this
  repo. Lockfile: only the root name/version lines change.
- Nodes Thinker and LmChatThinker, credential ThinkerApi (thinkerApi), base
  URL default http://localhost:2099.
- Both nodes refuse an API key that does not start with thk_ before sending
  any request.
- THINKER_ATTRIBUTION_HEADERS with User-Agent n8n-nodes-thinker; the backend
  caller classifier recognises the new UA.
- Provisional "T" logo until #10.
- dist/ removed from git and ignored; root nodes/ and credentials/ mirrors
  deleted.
- CHANGELOG restarted at 0.1.0; README documents npm pack and a manual
  community-node install. LICENSE.md keeps the MNFST, Inc. notice (MIT).

BREAKING CHANGE: node, credential and package names change; existing n8n
workflows must be rewired to the Thinker nodes and credential.

Closes #24

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* docs(dev): describe the unpublished thinker-cli package

Refs #23

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* fix(docker): keep the upgraded compose file readable (0644)

mktemp creates the replacement with mode 0600; the compose file carries
no secrets, so restore the usual 0644 after the atomic move. Found by
qa-docker.

Refs #26

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* chore(deploy)!: rebrand community deploy templates to Thinker

- render.yaml, app.json, .do/deploy.template.yaml, Dockerfile.heroku and
  deploy/* (fly, coolify, gcp, aws, easypanel, apple-containers, koyeb,
  digitalocean, heroku) use the image tmsoftbrasil/thinker:0.1 (Apple
  Containers follows THINKER_VERSION like Compose) and generate or ask
  for THINKER_ENCRYPTION_KEY; since #138 the backend refuses to boot
  with only MANIFEST_ENCRYPTION_KEY.
- Drop the dead MANIFEST_MODE everywhere, manifest.build links, the
  upstream logo and the AWS quick-create link to Manifest's S3 bucket.
- deploy/aws/manifest.yaml -> deploy/aws/thinker.yaml; Apple Containers:
  MANIFEST_ENV_FILE / MANIFEST_PG_VOLUME / MANIFEST_RECORDINGS_VOLUME ->
  THINKER_* (no aliases), container names thinker-postgres/thinker-app,
  Postgres digest read from docker/docker-compose.yml (Dependabot keeps
  that one current) with a synced fallback; Coolify Postgres pinned by
  digest.
- Every TUTORIAL.md starts with the PT-BR "community-maintained, not
  tested in CI" notice; root templates carry it as a comment.
- CONTRIBUTING.md: Thinker naming, thk_ keys, Docker Hub image and the
  tag-driven release flow; the stale Codecov badge goes.

BREAKING CHANGE: MANIFEST_ENV_FILE, MANIFEST_PG_VOLUME and
MANIFEST_RECORDINGS_VOLUME are no longer read by
deploy/apple-containers/start.sh.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* feat(frontend): rebrand the dashboard to Thinker (#25)

- Thinker name, logo (brand assets, cache-busted with ?v=1) and favicon on
  every page; index.html title and OG/Twitter tags with no absolute URLs.
- Setup guides: OpenCode, OpenClaw disable, MCP clients use the `thinker`
  provider/server name; n8n uses n8n-nodes-thinker; Craft points at a custom
  OpenAI-compatible endpoint; HeaderTierModal placeholder x-thinker-tier.
- Links: Docs and Help point to the Thinker repository (docs, issues);
  Manifest terms, privacy, support email, blog, discussion and the
  modelparams.dev report action are removed.
- Web Storage keys move to thinker*; pre-rename keys are deleted on load;
  lazy-reload survives blocked sessionStorage.
- Gateway marker `thinker` gets the Thinker icon and label.
- Routing pricing toast explains when catalog sync is off (#75 item 5).
- Playground headers: shared validators, warnings for duplicate and
  over-cap rows, announcement reset on close, adapter note (#105 items 4-5).
- Tests use thk_ keys and x-thinker-tier as data (#141 frontend part).
- Docker smoke checks <title>Thinker</title>.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* chore!: rename every remaining Manifest reference to Thinker

Sweep the identifiers, types, files, MCP tool names, routing reasons,
analytics fields, error messages, docs, URLs and test fixtures that still
carried the upstream brand, on top of the folded rebrand branches.

- MCP tools manifest_* -> thinker_*; MCP server name thinker.
- ManifestError/ManifestErrorCode/formatManifestError/ManifestRequest and
  friends -> Thinker*; files manifest-error, manifest-runtime and
  manifest-error-docs renamed.
- routing_reason manifest_* -> thinker_* with an append-only migration
  (RenameThinkerRoutingReasons1803600000000, up/down).
- origin=manifest filter -> origin=thinker; manifest_errors,
  manifest_lift_pct, manifest_error_code/message -> thinker_*.
- Error docs links point to docs/erros.md in the repository (new PT-BR
  page) instead of manifest.build; OpenRouter HTTP-Referer and email
  footers point to the repository; default dashboard URL is
  http://localhost:2099.
- Dev seed login admin@thinker.example.com / thinker.
- Templates use THINKER_RECORDING_* (backend side in #148) and the
  Compose file, Apple Containers script and Docker Smoke require
  THINKER_ENCRYPTION_KEY (#150).
- README links point to patrickbrandao/thinker; upstream-only badges,
  banners and the hosted cloud section removed.

Kept on purpose: upstream attribution, legacy guards (legacy-env,
legacy-storage, absence assertions), frozen storage-format salts,
applied migrations, and generic "manifest" meanings (OCI manifests).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* test(backend): no hosted gateway host left in backend production files

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* fix(backend): escape error text in SSE and friendly bodies, add nosniff

CodeQL flagged reflected XSS / exception text as HTML on the playground
SSE writer, the friendly SSE response and the proxy stream-error frames.
Those bodies now go through safeJsonStringify, which writes <, >, &,
U+2028 and U+2029 as \uXXXX escapes: the JSON every client parses is
unchanged. SSE responses and the friendly JSON response also send
X-Content-Type-Options: nosniff, and the stream-error path declares
text/event-stream when no header was sent yet.

Also drop the duplicated THINKER_BASE_URL and stale TODO(#31) in Docker
Smoke, and document that 0.1 is fresh-install only for Manifest users.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Gravações de requisição: retenção padrão de 30 dias

1 participant